I tried the fix in gitlab CI, it passed on 3 different versions of VCS.
I think that Risto already handled this for VCS, when he made it compatible with Ninja.
And now it correctly passes on xcelium, thanks for the fix!

Hello @nookfoo @mtravaillard ,

These cd commands in the SOCMAKE_SIM_RUN_CMD are quite ugly and really a workaround.
And now additionally you need to hack around that string insert the PYTHONPATH.

If there was a solution to get rid of the cd there it would be ideal.

I think we either need to always create the default target (remove the NO_RUN_TARGET option) and then let the user build on top of it (e.g., retrieve the working dir of the target and create it's own target) or use the hack we have now.

@nookfoo @mtravaillard any update? Can we find a nicer solution than using these cd commands?

I haven't found anything yet, cmake will use cd anyways to run some commands.

I haven't found anything yet, cmake will use cd anyways to run some commands.

Just to clarify why I had to add cd in the SOCMAKE_SIM_RUN_CMD

Its correct what you are saying that CMake normally adds cd to custom commands, but this is due to WORKING_DIRECTORY argument.

If someone uses the SOCMAKE_SIM_RUN_CMD later in theory they would have to set the WORKING_DIRECTORY on the custom command that executes this, but I don't know how to nicely transfer this information, so that's why I added the cd command.

Better option (cross-platform) would be to use ${CMAKE_COMMAND} -E chdir ${OUTDIR} ${run_sim_cmd} (https://cmake.org/cmake/help/latest/manual/cmake.1.html#cmdoption-cmake-E-arg-chdir).
But not sure if this would improve this issue in any way

I guess CMake does not expose the WORKING_DIRECTORY of a target as an accessible property. But what we could do is always create the base target and add our own custom property, for example:

set_property(TARGET ${run_target} PROPERTY TARGET_WORKING_DIRECTORY "${OUTDIR}")

Then we could retrieve this information later in the flow knowing the target name. What I don't really like is that it creates a target that you may not want/needed (maybe even not working on its own) by always creating the target.

Reviewing the code, I think we can remove the cd part from SOCMAKE_SIM_RUN_CMD. Instead, we should let the user pass the correct OUTDIR variable to the functions that use SOCMAKE_SIM_RUN_CMD.

In the cocotb function, this is simple because we directly call xcelium, so we already know OUTDIR. Almost nothing needs to be changed there.

In other projects, such as SoCs, we usually use this to add tests via CMake/CTest. In that scenario as well, we can easily determine the working directory because we typically pass it to the simulator function (e.g., xcelium or verilator).

Still, the OUTDIR argument is always optional, and we define it automatically if it is not provided. In this case, the simplest solution would be to also return SOCMAKE_SIM_RUN_DIR along with SOCMAKE_SIM_RUN_CMD. This way, each internal function can define whatever it needs, and the user receives the correct working directory.

The cd part can indeed be removed, it is not necessary to run the SOCMAKE_SIM_RUN_CMD (at least when using Xcelium, though we should make sure this works with the other tools as well).

Then, we could also return SOCMAKE_SIM_RUN_DIR, but all my current use cases are:

1. Adding tests with ctest . In this case, I want to run each test in a separate directory. Right now, I am actually stripping the cd part from SOCMAKE_SIM_RUN_CMD.
2. Internal use, such as in the Cocotb function, where we call a simulator but everything is managed internally.

For now, we could still return SOCMAKE_SIM_RUN_DIR to make things clearer/simpler for the user.

@Risto97 Any comment? If not, @mtravaillard could you try removing it and see if everything works?

Sorry, I haven't check yet.

The issue is that this change would break flows, as I have also in few places where I use this mechanism to add additional arguments to the simulation run command.

Your solution would work, but it is more complicated from user perspective, as if they don't take into account the SOCMAKE_SIM_RUN_DIR it will just not work.

Let me check this hopefully this week and I will see how easy it is to use this new variable.

Hello, I tried to play with this, and it does seem like it would not be crazy complicated to use this.
We can maybe proceed with the implementation and see if we can all update our repos to use this.

I have removed the cd and so far i haven't noticed any difference, when running the examples, for xcelium, vcs and questa.

I have also noticed, that for some simulator, for examples vcs.cmake, we still have both variable set, SOCMAKE_SIM_RUN_CMD and SIM_RUN_CMD :